[MWPW-153363] Countdown Timer implementation based on page metadata #2928

rahulgupta999 · 2024-09-20T10:56:30Z

As presented in AST, the regions have been using a ticker/timer/countdown visual to boost click-through rates and engagement as an enticement for certain promotions; e.g., Black Friday / Cyber Monday

Countdown Timer (CDT)
Properties:

Resolves: MWPW-153363

Test URLs:

Before: https://main--milo--adobecom.hlx.page/drafts/rahulgup/marquee-parent?martech=off
After: https://cdt-metatag--milo--rahulgupta999.hlx.page/drafts/rahulgup/marquee-parent?martech=off

This has the following authoring dependencies
New Placeholder texts are to be added in the placeholder file
{{cdt-ends-in}}
{{cdt-days}}
{{cdt-hours}}
{{cdt-mins}}

additionally, the following metadata should be present on the page
<meta name="countdown-timer" content="2024-08-26 12:00:00 PST,2024-09-30 00:00:00 PST">

Authoring documentation:
https://main--milo--adobecom.hlx.page/docs/authoring/promotions/countdowntimer

aem-code-sync · 2024-09-20T10:57:00Z

	Page	Scores	Audits	Google
📱	/drafts/rahulgup/marquee-parent?martech=off
🖥️	/drafts/rahulgup/marquee-parent?martech=off

codecov · 2024-09-20T10:57:48Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 96.27%. Comparing base (1791eb8) to head (4b93016).
Report is 2 commits behind head on stage.

Additional details and impacted files

@@            Coverage Diff             @@
##            stage    #2928      +/-   ##
==========================================
+ Coverage   96.24%   96.27%   +0.02%     
==========================================
  Files         236      238       +2     
  Lines       54278    54515     +237     
==========================================
+ Hits        52241    52485     +244     
+ Misses       2037     2030       -7

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

rahulgupta999 · 2024-09-20T12:48:03Z

I am waiting for access to promotions sheets to test this PR. For now, I have tested using injection.

npeltier

a few nits, but most of those changes are imo required

libs/blocks/hero-marquee/hero-marquee.js

libs/blocks/marquee/marquee.js

libs/blocks/media/media.js

libs/features/cdt/cdt.js

npeltier

lgtm

nit, in createTag, last param can be { parent: container }, and it will be appended to container which should make you save a few lines

vhargrave

Found a few things 👍
Let me know what you think.

libs/features/cdt/cdt.js

libs/blocks/marquee/marquee.js

libs/blocks/hero-marquee/hero-marquee.js

libs/features/cdt/cdt.js

vhargrave

nice!
i took a harder look at the cdt code as well this time and found these things. Once this is all fixed up should be good from my end.

libs/blocks/marquee/marquee.js

vhargrave · 2024-09-23T09:59:16Z

libs/blocks/marquee/marquee.js

  }
+
+  await Promise.all(promiseArr);


amazing, can you add a test example of a marquee with both mnemonic-list & countdown-timer to ensure this faster implementation also doesn't break anything?

vhargrave · 2024-09-23T10:05:33Z

libs/features/cdt/cdt.js

+  function render(daysLeft, hoursLeft, minutesLeft) {
+    if (!isVisible) return;
+
+    removeCountdown();


why always remove the countdown ? Can you not just update the timer label?

libs/features/cdt/cdt.js

afmicka · 2024-09-23T10:23:27Z

@rahulgupta999 @npeltier @yesil the countdown timer is not respecting its own start and end time when using instant parameter. Rather checks your browser time to decide whether to show the timer or not.

If you have a promo that lasts for some time (lets say 3 months) but want countdown timer to be shown only for a week inside that period, the current changes will show the timer as soon as the promo is active disregarding the defined timer starting date that comes later.

Here we have a promo that started in March this year, and timer is set to start in September. If we look at the page with the date before September with instant param, the countdown timer is still shown. The same applied for the timers end date.
https://cdt-metatag--milo--rahulgupta999.hlx.page/drafts/nala/features/promotions/promo-replace-with-countdown?instant=2024-04-22T05:00:00.000Z

cc: @Roycethan

libs/features/cdt/cdt.js

afmicka · 2024-09-23T11:55:52Z

it is fixed now. The rest looks good to me. RTL is also supported.

@rahulgupta999 could you please the failing check? I will mark this PR ready for stage

narcis-radu · 2024-09-23T11:51:33Z

libs/features/cdt/cdt.css

+}
+
+.horizontal .timer-block {
+  margin-left: 10px;


what about RTL support ? In RTL there's currently no space between text and timer

libs/utils/decorate.js

narcis-radu · 2024-09-23T12:07:59Z

libs/features/cdt/cdt.js

+}
+
+function isMobile() {
+  return window.matchMedia('(max-width: 767px)').matches;


The value gets cached and will not work when rotating screens. You need to refactor this to always see if the viewport matches the width.
Example:

const isMobile = window.matchMedia('(max-width: 767px)'); if (isMobile.matches) doSomething(); // this will work after you resize or rotate screens.

libs/features/cdt/cdt.js

libs/features/cdt/cdt.css

* Fix RTL issues with Countdown Timer * UT fix * review comment * review comment * review comment * font fixes * do not reverse * Revert "UT fix" This reverts commit c7e4d1d. * added CDT in notifications standard variant * ends in vertical positioning fix * review comments * UT added for cdt in notification * lint * css fixes * minor fix * lint * css * update timer for instant query param * narcis review feedback incorporated from PR #2928 * minor refactoring * fix CDT alignment in media and modal block --------- Co-authored-by: Rohit Sahu <[email protected]>

* Fix RTL issues with Countdown Timer * UT fix * review comment * review comment * review comment * font fixes * do not reverse * Revert "UT fix" This reverts commit c7e4d1d. * added CDT in notifications standard variant * ends in vertical positioning fix * review comments * UT added for cdt in notification * lint * css fixes * minor fix * lint * css * update timer for instant query param * narcis review feedback incorporated from PR adobecom#2928 * minor refactoring * fix CDT alignment in media and modal block --------- Co-authored-by: Rohit Sahu <[email protected]>

CDT Approach based on metadata

60ee7dc

rahulgupta999 added run-nala Run Nala Test Automation against PR commerce new-feature New block or other feature labels Sep 20, 2024

Add unit tests for CDT

b675101

rahulgupta999 requested review from yesil, npeltier, Axelcureno and 3ch023 September 20, 2024 12:46

rahulgupta999 marked this pull request as ready for review September 20, 2024 12:46

rahulgupta999 requested review from ryanmparrish, Sartxi, auniverseaway and a team as code owners September 20, 2024 12:46

rahulgupta999 requested review from narcis-radu and vhargrave September 20, 2024 12:50

rahulgupta999 mentioned this pull request Sep 20, 2024

[MWPW-153363] Countdown Timer - CDT #2803

Closed

rahulgupta999 changed the title ~~[MWPW-153363] Countdown Timer implementation based on Page Metadata~~ [MWPW-153363] Countdown Timer implementation based on page metadata Sep 20, 2024

rahulgupta999 added 5 commits September 20, 2024 19:05

self code review

0eb2417

removed comments

9eea2ee

Add support for CDT in media block

6274f78

Code Coverage fix

2186a05

code coverage

4ab0ebb

rahulgupta999 added the high priority Why is this a high priority? Blocker? Critical? Dependency? label Sep 20, 2024

code coverage

dd5fa31

Sartxi approved these changes Sep 20, 2024

View reviewed changes

npeltier requested changes Sep 20, 2024

View reviewed changes

Handling Review Comments

ee7a296

npeltier approved these changes Sep 20, 2024

View reviewed changes

rahulgupta999 added 5 commits September 20, 2024 23:21

Review comments

2a9199d

add unit test for code coverage

cebadd6

added cdt feature tests

f7734c8

increase code coverage

85cae8c

NIT for one line coverage

61733c9

vhargrave reviewed Sep 23, 2024

View reviewed changes

rahulgupta999 added 3 commits September 23, 2024 14:54

review comments for parallel load

6a73f13

code coverage

ad9149e

Fix UTC

5557a80

vhargrave reviewed Sep 23, 2024

View reviewed changes

npeltier reviewed Sep 23, 2024

View reviewed changes

libs/features/cdt/cdt.js Outdated Show resolved Hide resolved

Support instant query param

5eb6519

afmicka added verified PR has been E2E tested by a reviewer Ready for Stage labels Sep 23, 2024

Fix code coverage

4b93016

milo-pr-merge bot merged commit c5e352f into adobecom:stage Sep 23, 2024
14 checks passed

milo-pr-merge bot mentioned this pull request Sep 23, 2024

[Release] Stage to Main #2910

Merged

narcis-radu reviewed Sep 23, 2024

View reviewed changes

rahulgupta999 deleted the cdt-metatag branch September 23, 2024 12:56

rohitsahu pushed a commit to rohitsahu/milo that referenced this pull request Sep 27, 2024

narcis review feedback incorporated from PR adobecom#2928

398f172

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[MWPW-153363] Countdown Timer implementation based on page metadata #2928

[MWPW-153363] Countdown Timer implementation based on page metadata #2928

rahulgupta999 commented Sep 20, 2024 •

edited

Loading

aem-code-sync bot commented Sep 20, 2024 •

edited

Loading

codecov bot commented Sep 20, 2024 •

edited

Loading

rahulgupta999 commented Sep 20, 2024

npeltier left a comment

npeltier left a comment

vhargrave left a comment

vhargrave left a comment

vhargrave Sep 23, 2024

vhargrave Sep 23, 2024

afmicka commented Sep 23, 2024 •

edited

Loading

afmicka commented Sep 23, 2024 •

edited

Loading

narcis-radu Sep 23, 2024

narcis-radu Sep 23, 2024

[MWPW-153363] Countdown Timer implementation based on page metadata #2928

[MWPW-153363] Countdown Timer implementation based on page metadata #2928

Conversation

rahulgupta999 commented Sep 20, 2024 • edited Loading

aem-code-sync bot commented Sep 20, 2024 • edited Loading

codecov bot commented Sep 20, 2024 • edited Loading

Codecov Report

rahulgupta999 commented Sep 20, 2024

npeltier left a comment

Choose a reason for hiding this comment

npeltier left a comment

Choose a reason for hiding this comment

vhargrave left a comment

Choose a reason for hiding this comment

vhargrave left a comment

Choose a reason for hiding this comment

vhargrave Sep 23, 2024

Choose a reason for hiding this comment

vhargrave Sep 23, 2024

Choose a reason for hiding this comment

afmicka commented Sep 23, 2024 • edited Loading

afmicka commented Sep 23, 2024 • edited Loading

narcis-radu Sep 23, 2024

Choose a reason for hiding this comment

narcis-radu Sep 23, 2024

Choose a reason for hiding this comment

rahulgupta999 commented Sep 20, 2024 •

edited

Loading

aem-code-sync bot commented Sep 20, 2024 •

edited

Loading

codecov bot commented Sep 20, 2024 •

edited

Loading

afmicka commented Sep 23, 2024 •

edited

Loading

afmicka commented Sep 23, 2024 •

edited

Loading